Skip to content

fix: servient.shutdown does not close open http connections - #1548

Open
DhairyaMajmudar wants to merge 1 commit into
eclipse-thingweb:masterfrom
DhairyaMajmudar:1506-servient_shutdown
Open

fix: servient.shutdown does not close open http connections#1548
DhairyaMajmudar wants to merge 1 commit into
eclipse-thingweb:masterfrom
DhairyaMajmudar:1506-servient_shutdown

Conversation

@DhairyaMajmudar

Copy link
Copy Markdown

Description

Servient.shutdown() did not close active HTTP property observations. Although shutdown completed, long-polling or SSE connections could remain open and prevent the Node process from exiting.

The reason for this was that HTTP and HTTPS client factories did not retain the clients they created, and their destroy() methods did not stop those clients.

References

Closes: #1506
Related to: #1503 (comment)

cc: @relu91 @egekorkan

Signed-off-by: DhairyaMajmudar <dhairya.opensource@gmail.com>
public readonly scheme: string = "http";
private config: HttpConfig | null = null;
private oAuthManager: OAuthManager = new OAuthManager();
private readonly clients = new Set<ProtocolClient>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question about why using Set instead of an array. I wonder whether it can happen that a HttpClient may be treated as duplicate... hence creating for example 2 HttpClient instances but having just one entry in the set...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielpeintner , I used a Set to avoid accidentally tracking the same instance more than once, but I’m happy to use an array for consistency with MqttClientFactory if you prefer.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@relu91 any opinion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

servient.shutdown does not close open http connections

2 participants